-
Notifications
You must be signed in to change notification settings - Fork 1.2k
feat(grpc): make ChildManager call the parent work_scheduler #2443
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
f888c3b to
c8a2166
Compare
| // TODO: This is mainly provided as a fairly complex example of the current LB | ||
| // policy in use. Complete tests must be written before it can be used in | ||
| // production. Also, support for the work scheduler is missing. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: Probably need to remove todo about the missing work scheduler?
|
I started working on all the things you commented on here, but I realized now that pretty much all of your comments should be on #2442. (See PR description:)
(And then I made one more commit on top of that "final commit" and fixed the old comment that you noticed. So please focus on just https://github.com/hyperium/tonic/pull/2443/files/f933d877c87e97a6c18cb14a86a861f9f7efe6a6..3332da3710d62e4c5999c88c3dd4a2d3884665b5 for this PR and we can carry forward the other discussions in #2442.) |
e5f8520 to
915958d
Compare
This is based on top of #2442; please review only the final commit in this PR.
I found this pattern for writing tests to be painful. I will try to spend some time working on another approach.
The main problem I had with the tests was that the test_utils mod creates the
StubPolicyandStubPolicyDatameaning the tests have a hard time observing what the children Funcs did except by observing what was called on the child_controller or work_scheduler.